Skip to content

Do not expose sensitive informations (json format)#1024

Merged
dunglas merged 2 commits into
api-platform:2.0from
soyuka:security/json-error-normalizer
Apr 1, 2017
Merged

Do not expose sensitive informations (json format)#1024
dunglas merged 2 commits into
api-platform:2.0from
soyuka:security/json-error-normalizer

Conversation

@soyuka

@soyuka soyuka commented Mar 30, 2017

Copy link
Copy Markdown
Member
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #978 (comment)
License MIT
Doc PR na

#978 (comment)

@soyuka
soyuka changed the base branch from master to 2.0 March 30, 2017 15:33

trait ErrorNormalizerTrait
{
private function getErrorMessage($object, bool $debug = false)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

: string?

@soyuka
soyuka force-pushed the security/json-error-normalizer branch from 51079f6 to 238dbc1 Compare March 30, 2017 16:18
@jsamouh

jsamouh commented Mar 30, 2017

Copy link
Copy Markdown
Contributor

@dunglas @meyerbaptiste @soyuka , as I said in my comment in the previous PR (but nobody answered me ^^), does it work for a custom error normalizer ? If I decide to use XML, it won't work event with this PR ^^.

@soyuka

soyuka commented Mar 30, 2017

Copy link
Copy Markdown
Member Author

ErrorFormatGuesser should fallback to the first available if none is registered no?

if ($debug) {
return $message;
} elseif ($object instanceof FlattenException) {
$statusCode = $context['statusCode'] ?? $object->getStatusCode();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you forgot the $context var in the definition of your method 😉

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we have no tests for context \o/ thanks @meyerbaptiste

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved in last commit and I added a test

@jsamouh

jsamouh commented Mar 31, 2017

Copy link
Copy Markdown
Contributor

nobody to react to my comments ? :-(

@soyuka

soyuka commented Mar 31, 2017

Copy link
Copy Markdown
Member Author

I answered @jordscream isn't this what the ErrorFormatGuesser is supposed to do?

@soyuka
soyuka force-pushed the security/json-error-normalizer branch from 3710ce7 to 1a6362e Compare March 31, 2017 12:34
@dunglas
dunglas merged commit dfce158 into api-platform:2.0 Apr 1, 2017
@dunglas

dunglas commented Apr 1, 2017

Copy link
Copy Markdown
Member

Thank you very much @soyuka

@soyuka
soyuka deleted the security/json-error-normalizer branch April 1, 2017 07:24
hoangnd25 pushed a commit to hoangnd25/core that referenced this pull request Feb 23, 2018
…normalizer

Do not expose sensitive informations (json format)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants